Include window offset info in data section when building binaries for AMD platforms ACP_6_3 and ACP_7_0. #9707
Conversation
saisurya-ch
requested review from
sunilkumardommati,
lgirdwood,
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
There was a problem hiding this comment.
ok from Intel point of view, @bhiregoudar feel free to merge. One minor note inline.
| @@ -34,3 +34,4 @@ CONFIG_COMP_TDFB=n | |||
| #CONFIG_COMP_MUX=n | |||
| CONFIG_COMP_SEL=n | |||
| CONFIG_COMP_MIXER=n | |||
| CONFIG_BINARY_BUILD=n | |||
There was a problem hiding this comment.
Ths is a bit odd as there's no BINARY_BUILD Kconfig option defined in the tree. I guess this works,but is not the standard approach.
There was a problem hiding this comment.
nnno, let's not do this. All CONFIG_* symbols must have Kconfig entries. It becomes very confusing otherwise. Please add a Kconfig option or make it a generic macro, then of course you cannot define it in *_defconfig. Let me block this for clarification, I stand open for discussion of course
| @@ -34,3 +34,4 @@ CONFIG_COMP_TDFB=n | |||
| #CONFIG_COMP_MUX=n | |||
| CONFIG_COMP_SEL=n | |||
| CONFIG_COMP_MIXER=n | |||
| CONFIG_BINARY_BUILD=n | |||
There was a problem hiding this comment.
nnno, let's not do this. All CONFIG_* symbols must have Kconfig entries. It becomes very confusing otherwise. Please add a Kconfig option or make it a generic macro, then of course you cannot define it in *_defconfig. Let me block this for clarification, I stand open for discussion of course
…ries. Include window info in data section when building binaries for AMD platforms ACP_6_3 and ACP_7_0. Signed-off-by: SaiSurya Ch <saisurya.chakkaveeravenkatanaga@amd.com>
|
@lyakh good for you now ? |
| bool | ||
| default n | ||
| help | ||
| Select this if the platform need firmware binaries. |
There was a problem hiding this comment.
Sorry, not sure I understand the comment correctly. What's the alternative? What does CONFIG_AMD_BINARY_BUILD=n mean? No firmware binaries needed? But SOF is providing such a firmware "binary?" Or do you mean additional binary-only firmware parts?
There was a problem hiding this comment.
We meant to include the xsram_window in the data section using this config.
By default, xsram_window is present in the .fw_metadata section of rimage. But in a few scenarios, we need this data to be present in the data section of rimage.
We meant binary because, we like to parse few sections of the built rimage and make our own binaries in few cases, for which we need xsram_window to be present in data section rather than the fw_metadata section.
There was a problem hiding this comment.
@saisurya-ch maybe you could add a bit of information here in an incremental PR, if I were building SOF for AMD, I wouldn't be sure whether I need this option
lyakh
dismissed
their stale review
The comment has been addressed, thanks. Some documentation text could still be improved, but that isn't critical
Include window info in data section when building binaries for AMD platforms ACP_6_3 and ACP_7_0.